Skip to content

Conversation

@gianarb
Copy link
Contributor

@gianarb gianarb commented Dec 15, 2025

Related #15149

Motivation

mdspell is a bit too strict let's explore an alternative. I see
codespell has a good amount of starts and it looks maintained.

Modifications

Replace mdsell with codespell in Makefile

Verification

CI should be happy

@gianarb gianarb force-pushed the chore/try-codespell branch 3 times, most recently from a1e55d9 to 256d281 Compare December 16, 2025 17:26
Related argoproj#15149

mdspell is a bit too strict let's explore an alternative. I see
codespell has a good amount of starts and it looks maintained.

Signed-off-by: Gianluca Arbezzano <[email protected]>
@gianarb gianarb force-pushed the chore/try-codespell branch from 256d281 to d591ed9 Compare December 16, 2025 17:30
@gianarb gianarb marked this pull request as ready for review December 16, 2025 17:54
Copy link
Member

@Joibel Joibel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst I wouldn't mind using codespell for the code, it's not really suitable for checking documentation, it's not a spell checker in the traditional sense.

I quite liked the idea of using vale, but it might be difficult or too draconian on style until we'd updated all the docs to match kubernetes style. I don't have a correct answer here.

We'd also need to delete the .spelling file if we merged this PR.

$(TOOL_SNIPDOC) run

$(TOOL_MDSPELL): Makefile
# update this in Nix when upgrading it here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also remove these lines.

npm list -g [email protected] > /dev/null || npm i -g [email protected]
endif

$(TOOL_CODESPELL): Makefile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for duplicate lines here, just put the dependencies on the same line as the target.

@gianarb
Copy link
Contributor Author

gianarb commented Dec 17, 2025

Whilst I wouldn't mind using codespell for the code, it's not really suitable for checking documentation, it's not a spell checker in the traditional sense.

Makes sense, I didn't expect this to be the right approach, but it is a way to start the conversation.

The issue targets new-features.md and not the entire doc, so what I can do is to keep new-features.md out from mdspell check and figure out a different one for it?

Or I can add the words that mdspell complains about into the .spelling files so it will start to work again for it. Then I will add mdspell check for the various features make targets similar to what I did for the #15165 )

I think the last solution is the easiest and does not require new tooling. What do you prefer?

@Joibel
Copy link
Member

Joibel commented Jan 5, 2026

Whilst I wouldn't mind using codespell for the code, it's not really suitable for checking documentation, it's not a spell checker in the traditional sense.

Makes sense, I didn't expect this to be the right approach, but it is a way to start the conversation.

The issue targets new-features.md and not the entire doc, so what I can do is to keep new-features.md out from mdspell check and figure out a different one for it?

Or I can add the words that mdspell complains about into the .spelling files so it will start to work again for it. Then I will add mdspell check for the various features make targets similar to what I did for the #15165 )

I think the last solution is the easiest and does not require new tooling. What do you prefer?

Ok, lets spell check the individual features and we can switch to a better spelling tool another time then. I'd prefer fixes to the feature files over lots and lots of new words in .spelling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants